Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Save the subresults for tmt-report-result #3200

Merged
merged 27 commits into from
Oct 24, 2024
Merged

Save the subresults for tmt-report-result #3200

merged 27 commits into from
Oct 24, 2024
+395 −102

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Sep 9, 2024 •
edited
Loading

All the results generated by tmt-report-results become objects of tmt.result.SubResult and they get assigned under a parent tmt.Result. Also, set the TESTID and BEAKERLIB_COMMAND_REPORT_RESULT test environment variables, so every call of beakerlib's rlPhaseEnd macro becomes a subresult.

Related to:

Blocked by:

TODOs:

  • Try to find all the possible documentation places where this needs to be modified/mentioned - please provide tips.
  • Add tests
    • For beakerlib subresults
    • For shell subresults
    • Add tests for subresults logs reported by tmt-report-result -o <file>
    • Any more tests? Any ideas?
  • Fix existing tests / fix the code to pass tests
  • Any special handling for result: restraint? -> No. The subresults for test marked with result: restraint will be handled the same way as before (they get converted to "main" tmt custom results). The new thing here is that beakerlib phases are now also interpreted as tmt subresults. This means that in combination with result: restraint, the phases get also converted as main tmt custom results.
  • Missing subresults logs when tmt-report-result gets called and log file is provided as a third argument (rstrnt-report-result: TESTNAME TESTRESULT LOGFILE [METRIC] supports that). With -o everything works as expected. Is this a bug?
    • (answer from @happz ): tmt-report-result is our interface, we implement whatever we need to be there in a form we prefer and think is the best suited for the task of reporting results to tmt, and that comes with a set of options - it's not a 1:1 replacement for rstrnt-report-result.

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

@seberm seberm added enhancement area | results Related to how tmt stores and shares results labels Sep 9, 2024
@seberm seberm self-assigned this Sep 9, 2024
@seberm seberm added the ci | full test Pull request is ready for the full test execution label Sep 9, 2024
seberm
seberm commented Sep 9, 2024
View reviewed changes
tmt/schemas/results.yaml Outdated Show resolved Hide resolved
@seberm seberm added this to the 1.38 milestone Sep 10, 2024
seberm
seberm commented Sep 10, 2024
View reviewed changes
Copy link
Collaborator Author

@seberm seberm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to change the method docstrings, releases.rst and report-result.fmf story to reflect the subresults feature. Are there any other places where I should document this feature?

Let me know if you need any further adjustments / tests. Thanks.

tests/execute/result/subresults/test.fmf Outdated Show resolved Hide resolved
@seberm seberm force-pushed the feature/save-subresults branch 2 times, most recently from 918ef90 to 70d2fcf Compare September 16, 2024 09:36
@seberm
Copy link
Collaborator Author

seberm commented Sep 17, 2024

/packit build

@seberm seberm force-pushed the feature/save-subresults branch 3 times, most recently from 794f4fb to 701b4de Compare September 17, 2024 13:51
@psss psss removed the enhancement label Sep 17, 2024
@seberm seberm force-pushed the feature/save-subresults branch 2 times, most recently from 387636a to 821c8e1 Compare September 17, 2024 14:48
@seberm seberm marked this pull request as ready for review September 17, 2024 15:01
@seberm seberm marked this pull request as draft September 23, 2024 07:42
@seberm seberm force-pushed the feature/save-subresults branch 3 times, most recently from 428d10b to 9dff2f8 Compare September 25, 2024 15:41
@seberm seberm mentioned this pull request Sep 27, 2024
Open
seberm and others added 23 commits October 24, 2024 14:13
@seberm
subresults: small docs improvement
a62b022
@seberm
fixups and changes recommended by happz
c0a79e1
@seberm
remove data-path field for subresults
1b970dc
@seberm
also remove data-path for subresults in plan spec
952b57a
@psss @seberm
Let's keep consistent paragraph width
95733db
@seberm
Change 'parent result' to 'main result'
dc647ea
@seberm
Use plan.fmf instead of its definition in main.fmf
0684960
@seberm
Use rlRun_LOG variable instead of saving the output
a76759d
@seberm
Use pre-created tmp directory instead of /tmp
685f46c
@seberm
Add a /shell prefix to shell tests
08a605b
@seberm
Do not use one-letter variable name
58cdd18
@seberm
Do not add main result prefix to subresult name, keep it on framework
f2dd90e
@seberm
Just remove already reviewed FIXME
a4ac62a
@seberm
Remove explicit bash shell call for BEAKERLIB_COMMAND_REPORT_RESULT
62d659c
@seberm
Remove spurious empty line
e6fdef9
@seberm
Fix the ruff formatting
e981b85
@seberm
Fix another single-letter variable
5147d03
@seberm
Remove todos for check->subcheck conversion, seems to work correctly
234dfbd
@seberm
Use from_serialized/to_serialized to create SubResult from Result
2d54c75
@seberm
Get the SubResult options from the class instead of explicit definition
cd6f85e
@psss @seberm
Unnecessary set -o pipefail
ba82ef6
@psss @seberm
Wrap the long line
577e27b
@seberm
Add info about beakerlib subresults in combination with `result: rest…
a93fcc5 
…raint`
@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Oct 24, 2024
@psss psss merged commit 05a66e2 into main Oct 24, 2024
22 checks passed
@psss psss deleted the feature/save-subresults branch October 24, 2024 14:48
@psss psss self-assigned this Oct 24, 2024
@seberm seberm mentioned this pull request Nov 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psss psss psss approved these changes

@happz happz happz approved these changes

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@thrix thrix Awaiting requested review from thrix thrix is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

@martinhoyer martinhoyer Awaiting requested review from martinhoyer

Assignees

@psss psss

Labels
area | results Related to how tmt stores and shares results ci | full test Pull request is ready for the full test execution priority | must high priority, must be included in the next release status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Milestone
1.38
Development

Successfully merging this pull request may close these issues.
4 participants
@seberm @psss @happz @martinhoyer